This one isn't better. My first review wasn't particularly precise in what was bothering me, though: I do not consider this to be a “hooked property”, since there is no actual hook. It just borrows the hook syntax to define the operations required by the interface, so calling this a “hook” is wrong in my book.

Just dropping the “Hooked” from the error message might be enough.

In fact, the correct error message in this case would be:

Fatal error: Non-abstract property hook must have a body in %s on line %d

Since that would be emitted when the readonly is removed. The readonly isn't the main problem here, the missing hook body is.

The readonly isn't the main problem here, the missing hook body is.

Not sure if I am following here. In 8.4 we have:

Hooked properties cannot be 'readonly'
Non-abstract property hook must have a body

In 8.5 we would have:
Hooked properties in abstract classes cannot be declared readonly
Non-abstract property hook must have a body

I feel like I am testing exactly what I should test. I also feel the new error is perfectly fine.

An interface/abstract class declaring get or set doesn't imply hooks, just that those operations be valid. A normal, non-hooked property would satisfy that just as well.

I think the error messages should be something like:

interface I {
  public readonly $foo { get; }
}

"Interface properties may not be readonly."

abstract class A {
  abstract public readonly $foo { get; }
}

"Abstract properties may not be readonly."

class C {
  public readonly $foo { get; }
}

"Non-abstract property hook must have a body."

(Arguably we could combine the first two into a single message about abstract properties. I don't care much either way.)

Tim, would that work for you?

Tim, would that work for you?

Yes. The important bit for me is the priority of the error messages. The missing body is the actual issue, not the readonly. This should likely also fixed in PHP 8.4, since it's confusing.

Yes. The important bit for me is the priority of the error messages. The missing body is the actual issue, not the readonly. This should likely also fixed in PHP 8.4, since it's confusing.

I pushed an update.

Concern: To me it looks like Ilija did chose the previous order so that it would error out early before even looping over hooks->children. Now the loop runs always. Wdyt?